Skip to content

Conversation

@mascip
Copy link

@mascip mascip commented May 21, 2013

Hi Buddy,
following our email exchange of the 25/04, i added the possibility to define an Alias for the debuggit() function.

Tests are in t/alias.t, and i added tests in t/fallthrough.t and t/override.t too.
The feature is documented, both in Debuggit.pm and Debuggit::Manual (i didn't manage to read them in their final format, as i don't know how they're created by Metacpan).

If you're happy with that you can use it. If not, tell me why. I won't be offended, whatever the reason. I'm mostly doing this to learn: it's my first pull request with code in it :-)

If you like it, i might try and implement the other feature we spoke about: a DEBUGGIT_DEBUG environment variable, and the documentation that gives ideas on how to use it.

Good day to you :-)
Pierre Masci (mascip)

Edit: I'm happier with the latest commit. Much more maintainable i think: it's all in one small place. As you can see i'm learning as i'm going along ;-) Tell me if that's good enough.

@barefootcoder
Copy link
Owner

Hey Pierre.

I'm so sorry it's taken so long to get back to you. I left my old job shortly after we last spoke and am just now getting settled in to a new position.

Anyway, I've reviewed the work you've done here, and I wanted to clarify something. The way you've coded it, if you request an alias for debuggit, you end up with both the alias and debuggit. I wonder if this would accord with what the user expects. Might they not expect to get the alias instead of debuggit? That is, if they dislike the name debuggit so much they want to change it, they probably don't want a debuggit symbol exported to their namespace at all. :-)

Thus, I wonder if it might make more sense to just replace the default name with the alias. IOW, something along the lines of

    my $debuggit_name = $opts{Alias} || 'debuggit';

(I'd use // there, but then we'd lose compatibility with pre-5.10 perls) and then later simply:

         *{ join('::', $caller_package, $debuggit_name) } = \&debuggit;

What do you think?

@mascip
Copy link
Author

mascip commented Sep 24, 2013

That makes a lot of sense Buddy. I'll change it in the next few days.

All the best for your new job :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants